@janos could you verify that this is not impacting the way we build our docker images? If it is, then we should address this as well prior to merging this, so that we don't introduce issues during the release scheduled for 17 July.

It's missing some files. I had the same problem during the repo migration. As a workaround I had to copy these over manually to the vendor dir:

cp -R ../../ethereum/go-ethereum/crypto/secp256k1/libsecp256k1 vendor/github.com/ethereum/go-ethereum/crypto/secp256k1/libsecp256k1

Yes, I will @nonsense. And thanks @skylenet for the fix. This manual step is strange indeed.

There are problems installing linters by gometalinter.v2. This package is deprecated (even so --install command is deprecated earlier), its repo set to read-only and recommends to use github.com/golangci/golangci-lint instead. This looks like a blocker for running go run build/ci.go lint with modules. Should we try to switch to golangci-lint?

@janos sure, let's try a more recent linter then!

The main problem is vendoring cgo dependencies, directories that do not contain any go code, but C files golang/go#26366. We have github.com/karalabe/hid/hidapi and go-ethereum/crypto/secp256k1/libsecp256k1. This packages cannot be vendored with go modules without any additional tooling or scripting, which is very unfortunate. Even on calling go mod vendor, manually vendored files are deleted. I am not sure if it is nice to introduce new scripts or tools to handle this problem if govendor is still useful to us.

Another solution that I like is this one golang/go#26366 (comment), but it requires patches to third party libraries. Adding a blank import to a packages with C code and an empty go file.

Is this worth the trouble to use go modules?

@janos I think it's worth to fix the third party libs with the blank import. I'm sure that Peter wouldn't mind merging these quickly.

Thanks @skylenet, will update README.md, I missed that.

Vendoring directories that are not go packages can be done with "_" import, but if there are a lot of this directories it is very ugly, like in the situation with go-ethereum/crypto/secp256k1/libsecp256k1/libsecp256k1. This directory is a copy of the third party C library source tree and in order to keep it vendored, every directory needs to have a go file and each of those trivial go package needs to have a blank import. Adding go files makes maintaining of this C library source tree more difficult and I am not sure that it is a good idea.

Another option is to use go modules, but not to vendor packages. I like to have all packages vendored to be able to build offline, but that creates a problem with vendoring cgo dependencies in subpackages. If packages are not vendored and go tool uses packages from its local modules cache (requires fetching them if missing locally), then the whole module source tree is available and there should be no problem with cgo. Would such option be acceptable @skylenet @nonsense?

I do not see that go modules will ever vendor cgo dependencies as this is already recognized as bad practice from build perspective https://godoc.org/cmd/cgo golang/go#26366 (comment).

Note that changes to files in other directories do not cause the package to be recompiled, so all non-Go source code for the package should be stored in the package directory, not in subdirectories.

@janos I would really like to stick with having the vendored libs inside version control to avoid surprises whenever a third party lib deletes their repo or changes their git history.

What do you think about adding a vendor target to the Makefile that does the go modules vendoring part + copying over the missing ethereum/go-ethereum/crypto/secp256k1/ directories ?

@skylenet actually, go modules proxies (such https://proxy.golang.org/) should minimize the risk of library disappearing. Even so, this proxy.golang.org will be de default GOPROXY value in Go 1.13.

I do not like so much wrapping go tools if go tools can reverse the action from Makefile, like just calling go mod vendor, in which case an extra attention would be required when committing changes. I am still in favour not to vendor and to rely on public infrastructure and local caches, then to introduce maintenance complexity.

I don't have an opinion on how we want to maintain our vendored libraries (go-vendor, go modules, or any other tool), but I think it is very important to have a local copy of all the source needed to build Swarm.

If we are to maintain a local cache, I am not sure how this is decreasing complexity - it seems the opposite to me, but I am free to be convinced otherwise.

I would be against not having a copy of our dependencies and relying on external parties to function, so that we can build Swarm.

Offtopic: I had the same disagreement at a previous company, where it was out of my hands, and we opted for fetching dependencies from public infrastructure - the result was production going down 2 times in a span of 6 months because of missing dependencies 🔥

@nonsense I see your point about vendoring. Your experience with you previous company, is a very common thing to have if you have only one repository source, but I think that go proxies are a nice improvements on that. By local caches I mean local modules caches that we have on local machines where we do development, not any arbitrary cache that needs maintenance, from where additional redundancy is provided. Go modules proxies keep such caches for public use. Many companies have their own dependency caches (proxies) internally, like nexus, to avoid situations like you experienced.

@nonsense If you insist on vendoring, what to you want to have govendor or go modules with custom wrapping, or something else?

@janos I think I answered this with my comment above. I don't really want to be leading this change, I just expressed an opinion about vendoring, similar to what @skylenet wrote.

I think this change is nice, but doesn't really solve any critical problems that we have. If there is no option to keep all prerequisite source code for building Swarm within the Swarm repo, and this change comes at that cost, we have to make sure that the problems it addresses are worth it.

@nonsense Thanks, I am sorry for being so annoying. How would you like to proceed in this change? That is what I wanted to ask. Should we drop go modules based on the problems? You explained nicely your opinion about vendoring, but I asked about your preferred tooling, should we stick with govendor, or wrap go modules?

@janos please go ahead with the solution you think will give us the least maintenance work. @skylenet and @nonsense expressed preference for expicit vendoring. If this can be replaced easily but you think still provides security let's go for it.

As discussed in a call with @janos . We should proceed with having a vendor target in the Makefile that deals with the go-modules stuff and then copies over the missing C files.

If someone accidentally doesn't use the vendor target, we should still notice it on PR's, because you'll see that the C files will be removed.

Thanks @skylenet.

Maybe @acud or @holisticode could have a look at this. I think this should be merged "soon"™